-
Notifications
You must be signed in to change notification settings - Fork 184
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature write float32array #106
base: master
Are you sure you want to change the base?
Conversation
src/geotiffwriter.js
Outdated
@@ -252,11 +252,12 @@ const encodeImage = (values, width, height, metadata) => { | |||
|
|||
const prfx = new Uint8Array(encodeIfds([ifd])); | |||
|
|||
const img = new Uint8Array(values); | |||
// This should allow other kinds of typed arrays to be used. Uint8 is just a view of the real underlying data. | |||
const img = new Uint8Array(values.buffer); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, values is sometimes a simple JavaScript array and not a buffer, so there is no .buffer
property available in that case. Here's where this simple array is created: https://github.com/geotiffjs/geotiff.js/blob/master/src/geotiffwriter.js#L320
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, on reviewing the code I wrote, I can see it's super confusing and it's not clear what type something is. I'm definitely open to have it being re-written to be more clear as long as people can still pass in a simple array :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- When it is a normal js array, does it assume that it is 8 bits?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@m0ose yes, that is correct. That is an oversight on my part. My bad. Perhaps, you need to write some code to read metadata.BitsPerSample
and metadata.SampleFormat
and set what typed array to use based on that? I believe for Float32, BitsPerSample
should equal 32
and SampleFormat
should equal 3
. @constantinius might have insight of the tiff tags, too.
References:
Thanks @m0ose for the effort! Please keep in mind, that apart from writing the array-data several metadata tags have to be set in accordance to the metadata. For example the SampleFormat tag, that tells readers how to interpret the array data. |
It now writes metadata. There may be a problem 'though. If the metadata(bitsPerSample and sampleFormat) and the typed array do not match, should it go by whats in the metadata or the typed array? |
I think this should be solved using a layered architecture. The high level one should be as easy and uncomplicated as the The low level API should allow the setting of the various TIFF tags and raster data. Here it could be possible to set conflicting options, but users should usually not access that API directly. I consider the current write API as the lower level one, so you could pick where you get the metadata from and what to do when they are conflicting. |
woops. Fixed duplicated float32 check.
Hi, @m0ose . Let me know when you want me to take another look at your PR! Thanks for your awesome work :-) |
Ok. I think there are just some small things. I will try to get to them
soon.
Cody Smith
…On Tue, Oct 15, 2019 at 9:21 PM Daniel J. Dufour ***@***.***> wrote:
Hi, @m0ose <https://github.com/m0ose> . Let me know when you want me to
take another look at your PR! Thanks for your awesome work :-)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#106?email_source=notifications&email_token=AADADQXHV3L4BLDAKLJ6GVTQO2CCNA5CNFSM4I4MDQT2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBK4TGI#issuecomment-542493081>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADADQQOZ65YZUOJMAE4THLQO2CCNANCNFSM4I4MDQTQ>
.
|
Hey, this might be a unique problem to me? But When I tried your code it didn't work until I enforced big-endian encoding. So I changed the code like this (SUPER SUPER UGLY, just proof of working): const data = new Uint8Array(numBytesInIfd + (values.length * 4 * samplesPerPixel));
times(prfx.length, (i) => { data[i] = prfx[i]; });
// forEach(img, (value, i) => {
// data[numBytesInIfd + i] = value;
// });
// store data
for (let i = 0, vl = values.length; i < vl; i++) {
const value = values[i]
const buffer = new ArrayBuffer(4)
const view = new DataView(buffer)
view.setFloat32(0, value, false)
const uint8 = new Uint8Array(view.buffer)
const idx = numBytesInIfd + i * 4
data[idx] = uint8[0]
data[idx + 1] = uint8[1]
data[idx + 2] = uint8[2]
data[idx + 3] = uint8[3]
} |
After trying the above code, I am getting "RangeError: Offset is outside the bounds of the DataView" while parsing the arraybuffer returned. |
Resolved by adding correct "StripByteCounts" value along with "SampleFormat" and "BitsPerSample" |
trying to write float32bit the file is corrupted @manivannan23k can you help me.
|
This would be a fantastic addition. These tricky problems are above my ability but how can I best encourage progress? Is sponsoring the feature an option? It seems like a few people have been interested in this over time. |
Added support for float32Array writing. I only tested it on one float32Array but it should work on other typed arrays. Addresses This issue #103.
I tested it with this code.